test(scoring): cover pending-pr-scenarios' ghost-login and case-insensitive matching#8528
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…sitive matching sameLogin's `value &&` short-circuit exists for PullRequestRecord.authorLogin being string | null | undefined (ghost/deleted GitHub accounts), and both helpers lowercase before comparing because GitHub treats owner/repo and logins case-insensitively while stored values are not guaranteed consistent. Every existing test passed a concrete, same-cased login, so neither behavior was pinned. Adds one case covering all four: null and undefined authorLogin are excluded, and records differing only in repo-name or login case still match. The mock returns a review keyed to the requested pull number, so the assertion reads exactly which records survived the filter. Closes JSONbored#8329
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 18:39:21 UTC
Review summary Nits — 2 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
src/scoring/pending-pr-scenarios.ts's two filter helpers had untested defensive behavior:sameLogin'svalue &&short-circuit exists becausePullRequestRecord.authorLoginisstring | null | undefined— a real case for ghost/deleted GitHub accounts. Without it, anulllogin would throw on.toLowerCase().Every existing test passed a concrete, identically-cased login and repo name, so neither behavior was pinned.
Adds a single case covering all four situations the issue lists, asserting through
loadContributorRepoOpenPrSignalRecords:authorLogin: nullauthorLogin: undefinedrepoFullName: "Entrius/Allways-UI"vs query"entrius/allways-ui"authorLogin: "Miner-A"vs query"miner-a"The mock returns a review keyed to the requested pull number, so the assertion reads back the exact set of records that survived the filter (
[82, 83]) rather than inferring inclusion from an array length.Verified the test pins the behavior, rather than passing alongside it — each helper was reverted independently and the test failed both times:
sameLoginshort-circuit →(value as string).toLowerCase()sameRepoFullNamenormalization →left === rightCloses #8329
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
tsc --noEmitis clean andgit diff --checkpasses.codecov/patchhas no changed production lines to score. The scoped coverage run emits a non-emptycoverage/lcov.infocontainingsrc/scoring/pending-pr-scenarios.ts(the suite imports it directly), satisfying the "Verify coverage report exists" step.test/unit/pending-pr-scenarios.test.tspasses in full — 18 tests.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — a unit-test addition; no visible UI, frontend, docs, or extension change.
Notes
authorLogin: nullis passed asnull as unknown as stringbecause thepr()fixture'sPartial<PullRequestRecord>narrows it, while the underlyingPullRequestRecord.authorLoginreally isstring | null | undefined— the cast reproduces the genuine runtime shape the helper guards against, without loosening the fixture for every other test.